Skip to content

Conversation

@asir66
Copy link
Contributor

@asir66 asir66 commented Dec 15, 2025

Overview

This update is based on PR #245 and focuses on improving the robustness of cargo-optee in several areas, including manifest path resolution, std/no-std argument semantics, and consistency across documentation and CI.

Specifically, this PR includes the following changes:

  1. Fix issues in cargo-optee related to manifest path resolution, including workspace scenarios
  2. Clarify and improve the semantics and parsing logic of --std / --no-std
  3. Normalize JSON file field ordering
  4. Update the Quick Start documentation and CI configuration
  5. Summarize differences between cargo, cargo-sgx, and cargo-optee based on investigation, and outline potential future directions

Related background discussion: #245 (comment)

1. Manifest Path Resolution Fix

This issue is fundamentally caused by insufficiently robust manifest path resolution. It manifests differently depending on the project structure:

  1. Single-package projects
    When --manifest-path Cargo.toml (without ./) is explicitly provided, cargo-optee does not correctly handle this edge case when deriving the project path, which may result in failing to locate the project directory.

  2. Workspace projects
    When a relative path containing ./ is used as the manifest path, cargo metadata returns canonicalized manifest paths, while cargo-optee attempts to match them using the original, non-normalized path string. This mismatch causes package discovery to fail.

Fix and Rationale

To address this issue at its root, manifest paths are now canonicalized at the entry point using:

std::fs::canonicalize()

This standard library function normalizes paths into absolute form and automatically eliminates:

  • ./
  • ../

2. --std / --no-std Argument Semantics

Previous Behavior

cargo-optee previously used a boolean flag to represent whether std was enabled.
When the flag was not explicitly provided, it defaulted to Some(false), which
caused cargo-optee to always fall back to the no-std build mode.

Updated Design

This update introduces an explicit --no-std flag and refactors the argument
parsing logic accordingly:

  • --std and --no-std are mutually exclusive
  • If neither is specified, the internal state is set to None
  • The final build mode is determined according to the defined precedence rules

This clarifies user intent and improves extensibility for future configuration
sources.

3. JSON File Normalization

Relevant JSON files have been updated to ensure that fields are ordered in
ascending ASCII order.

  • This change does not affect functionality, but improves:
  • file consistency
  • diff readability

alignment with common tooling and formatting conventions

4. Documentation and CI Updates

  • The Quick Start guide has been updated to recommend the cargo-optee workflow as the primary approach
  • Documentation sections that were inconsistent with current behavior have been corrected
  • The CI configuration has been updated to cover the new arguments and corrected behavior

During this process, an additional documentation inconsistency was identified and addressed as part of this PR.

5. Investigation: Differences Between cargo, cargo-sgx, and cargo-optee

The following section summarizes findings from an investigation into the design and usage differences between cargo, cargo-sgx, and cargo-optee.

The content in this section is exploratory and design-oriented, and does not affect the functional fixes or merge decision of this PR.

5.1 cargo-optee Is Not a cargo Subcommand

Currently, cargo-optee cannot be invoked as a cargo subcommand (i.e. cargo optee).

One possible reason is that the customized cargo toolchain used in the environment does not include subcommand extension support.

From a pragmatic perspective, the current form is acceptable. However, from a consistency and extensibility standpoint, retaining this capability may be worth considering in the future.

5.2 Rationale for cargo-optee build ta/ca

Unlike cargo-sgx, which does not require explicit sub-target selection, cargo-optee requires specifying either ta or ca.

This difference stems from fundamental architectural distinctions:

  • In Intel SGX, enclaves are closer to being linked as libraries
  • In TrustZone, Trusted Applications are standalone secure-world entities

As a result, this design choice is both necessary and appropriate.

5.3 Default Build Mode Differences

Tool | Default Build Mode -- | -- cargo | `--debug` cargo-sgx | `--release` cargo-optee | `--release`

cargo-optee aligns with cargo-sgx in this respect, which matches the performance and determinism requirements of TEE environments.

5.4 run vs sync

Both cargo and cargo-sgx support running applications directly via run. In contrast, cargo-optee requires an explicit sync step before execution.

This difference is driven by runtime environment constraints:

  • SGX relies on real hardware and encapsulates the toolchain tightly
  • TrustZone requires explicit deployment of TA and CA artifacts to either an emulator or a physical device

Therefore, this behavior reflects architectural constraints rather than a tooling deficiency.

That said, it may be worth exploring whether the sync step could be optionally integrated into cargo-optee (e.g. via a flag or default behavior) to improve usability.

5.5 Features, Targets, and Workflow Overlap

In cargo:

  • features control code paths
  • targets select toolchains

In cargo-optee, these dimensions partially overlap at the workflow level.

This has also been a key point in previous discussions(see: #245 (comment)).

At present, the main source of complexity comes from the overlap between features and targets in the build workflow. This overlap makes certain parts of the model harder to reason about and evolve.

It is possible that this issue could be addressed more cleanly in the future if the toolchain converges on a cargo-only workflow (for example, if xargo is eventually phased out). Under such conditions, the separation of concerns between features and targets may become clearer.

For now, however, cargo-optee remains a well-suited tool for this project: it effectively reduces configuration burden, integrates smoothly with the TrustZone SDK, and provides a simple and efficient developer experience.

6. Future Directions (Design Considerations)

In the long term, cargo-optee could consider adopting a unified and configurable parameter resolution model, for example:

CLI arguments > Cargo.toml > environment variables > defaults

(Environment variables may also be viewed as overridable defaults.)

Such a model could:

  • reduce the need for verbose command-line invocations
  • improve build reproducibility
  • better align cargo-optee with Cargo’s configuration philosophy

This would allow for unified configurability without sacrificing explicitness.

Thanks the help of @DemesneGH @m4sterchain

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces cargo-optee, a new Rust-based build tool for OP-TEE Trusted Applications (TAs) and Client Applications (CAs), along with comprehensive documentation updates. The tool aims to replace complex Makefile-based workflows with a unified, type-safe command-line interface.

Key changes:

  • Implementation of cargo-optee with support for TA/CA/Plugin builds across aarch64/arm architectures and std/no-std modes
  • Fixed manifest path resolution issues for single-package and workspace projects
  • Improved --std/--no-std argument semantics with explicit mutually exclusive flags
  • Updated documentation to recommend cargo-optee as the primary workflow while maintaining backward compatibility with Makefile-based builds

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
cargo-optee/src/main.rs CLI interface with subcommands for build/install/clean operations
cargo-optee/src/ta_builder.rs TA build pipeline: clippy, build, strip, and sign
cargo-optee/src/ca_builder.rs CA/plugin build pipeline: clippy, build, and strip/copy
cargo-optee/src/config.rs Configuration resolution with priority: CLI > Cargo.toml metadata > defaults
cargo-optee/src/common.rs Shared utilities for architecture handling, command execution, and file operations
cargo-optee/Cargo.toml Dependencies for the cargo-optee tool
cargo-optee/README.md Comprehensive documentation for cargo-optee usage and design
cargo-optee/*.json Target specification files for custom aarch64/arm-unknown-optee targets
ci/build.sh New CI build script using cargo-optee to build all examples
.github/workflows/ci.yml Added CI jobs for testing cargo-optee builds in std/no-std and aarch64/arm32 configurations
docs/emulate-and-dev-in-docker.md Updated to document both cargo-optee and Makefile workflows
docs/emulate-and-dev-in-docker-std.md Added cargo-optee workflow alongside legacy switch_config approach
docs/ta-development-modes.md Corrected list of examples excluded in no-std mode
optee-utee-build/src/linker.rs Fixed environment variable name from TARGET_TA to TARGET
examples/metadata.json Newly added metadata file mapping examples to their TA/CA/plugin paths
.licenserc.yaml Updated to exclude all JSON files from license checking
README.md Added reference to cargo-optee documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DemesneGH
Copy link
Contributor

Please also fix the CI, thanks!

@asir66
Copy link
Contributor Author

asir66 commented Dec 16, 2025

Please also fix the CI, thanks!请同时修复 CI,谢谢!

Sorry, it's my fault. I will fix the CI and doc. thank you.

@asir66
Copy link
Contributor Author

asir66 commented Dec 17, 2025

@DemesneGH Hi , I’ve updated the relevant files according to your suggestions and fixed the CI issues.
Would you mind taking a look and reviewing the changes?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ivila
Copy link
Contributor

ivila commented Dec 29, 2025

For 5.1, "cargo-optee" is not recognized as a cargo subcommand, it is because cargo invokes external binaries in an unexpected way.

For example, cargo optee build is invoked as cargo-optee optee build. This extra "optee" argument causes the subcommand parser to fail.

One workaround is to add a parsing rule or configuration option that discards the first (duplicate) argument:

#[derive(Parser)]
#[command(version, about, long_about = None)]
struct Cli {
    // By stripping the duplicate "optee" argument during subcommand invocation, 
    // this solution maintains compatibility between standalone and cargo-integrated execution modes.
    #[arg(name = "tool_name", hide = true)]
    _tool_name: String,
    #[command(subcommand)]
    commands: Commands,
}

@asir66
Copy link
Contributor Author

asir66 commented Jan 9, 2026

For 5.1, "cargo-optee" is not recognized as a cargo subcommand, it is because cargo invokes external binaries in an unexpected way.

For example, cargo optee build is invoked as cargo-optee optee build. This extra "optee" argument causes the subcommand parser to fail.

One workaround is to add a parsing rule or configuration option that discards the first (duplicate) argument:

#[derive(Parser)]
#[command(version, about, long_about = None)]
struct Cli {
    // By stripping the duplicate "optee" argument during subcommand invocation, 
    // this solution maintains compatibility between standalone and cargo-integrated execution modes.
    #[arg(name = "tool_name", hide = true)]
    _tool_name: String,
    #[command(subcommand)]
    commands: Commands,
}

That makes sense—thank you for the suggestion.

I also noticed that with the previous implementation, running cargo-optee in standalone mode results in the following error:

error: the following required arguments were not provided:
  <tool_name>

For this reason, I believe the following approach is more robust: making _tool_name an optional argument.

#[derive(Debug, Parser)]
#[command(version, about)]
pub struct Cli {
    // By stripping the duplicate "optee" argument during subcommand invocation,
    // this solution maintains compatibility between standalone and cargo-integrated execution modes.
    // When invoked as `cargo optee build`, cargo passes "optee" as the first argument.
    // When invoked as `cargo-optee build`, no extra argument is passed.
    #[arg(name = "tool_name", hide = true)]
    _tool_name: Option<String>,

    #[command(subcommand)]
    pub cmd: Command,
}

This allows the CLI to work correctly in both invocation scenarios without requiring additional conditional logic.

@DemesneGH
Copy link
Contributor

@asir66 Thanks for your effort!
I've checked the implementation in cargo-sgx:

#[derive(Debug, Parser)]
#[clap(bin_name = "cargo")]
#[clap(version = VERSION)]
pub(crate) enum Opts {
    /// Utilities to build sgx program.
    #[clap(name = "sgx")]
    #[clap(version = VERSION)]
    #[clap(action = ArgAction::DeriveDisplayOrder)]
    Sgx(SgxArgs),
}

#[derive(Debug, clap::Args)]
pub(crate) struct SgxArgs {
    #[clap(subcommand)]
    cmd: Command,
}

#[derive(Debug, clap::Subcommand)]
enum Command {
    #[clap(name = "build")]
    Build(BuildCommand),
    #[clap(name = "run")]
    Run(RunCommand),
...
}

It seems cargo-sgx only supports being invoked as cargo-sgx sgx xxx and doesn't support the two scenarios.

Could you help with a brief study of how other cargo subcommands handle this case?

To achieve the goal of "cargo-optee acting as a cargo subcommand," it's good to follow the established principles used by other subcommands. If there is a standard convention, we can implement it in the same way; otherwise, the current implementation is fine.

@asir66
Copy link
Contributor Author

asir66 commented Jan 9, 2026

After reviewing a more canonical Cargo subcommand implementation, I noticed that cargo-fmt handles the Cargo-injected subcommand argument in a different and more robust way.

In cargo-fmt, the implementation explicitly drops the first occurrence of the subcommand name (fmt) at program entry, rather than relying on a positional placeholder in the argument parser. This behavior is intentional and aligns well with Cargo’s external subcommand invocation model.

When Cargo invokes an external subcommand, it follows the convention:

cargo <subcommand> <args...>
→ cargo-<subcommand> <subcommand> <args...>

As a result, a well-behaved Cargo subcommand should ideally support all of the following invocation forms:

  • cargo <args...>
  • cargo- <args...>
  • cargo- <args...>

I verified that cargo-fmt conforms to this convention, while cargo-sgx currently does not handle all of these cases consistently.

For reference, cargo-fmt implements this logic by filtering out the first occurrence of the injected subcommand name at the very beginning of execution:

fn execute() -> i32 {
    // Drop extra `fmt` argument provided by `cargo`.
    let mut found_fmt = false;
    let args = env::args().filter(|x| {
        if found_fmt {
            true
        } else {
            found_fmt = x == "fmt";
            x != "fmt"
        }
    });

    let opts = Opts::parse_from(args);
}

Following the same rationale, I propose adopting an equivalent approach for cargo-optee, by removing the first occurrence of "optee" from the argument list at program entry:

// Drop extra `optee` argument provided by `cargo`.
let mut found_optee = false;
let filtered_args: Vec<String> = env::args()
    .filter(|x| {
        if found_optee {
            true
        } else {
            found_optee = x == "optee";
            x != "optee"
        }
    })
    .collect();

let cli = Cli::parse_from(filtered_args);

This approach avoids introducing Cargo-specific positional arguments into the CLI definition, keeps the argument parsing logic explicit and localized, and ensures consistent behavior across all supported invocation forms.

I believe this pattern is both more idiomatic and better aligned with existing Cargo tooling practices, as demonstrated by cargo-fmt.

@asir66
Copy link
Contributor Author

asir66 commented Jan 9, 2026

This commit performs a structural refactor of cargo-optee to improve
consistency, readability, and long-term maintainability.

Key changes include:

  • Extract CLI-related data structures from the main module into a dedicated
    cli.rs module, clarifying responsibilities and improving modularity.

  • Normalize cargo subcommand behavior by stripping the duplicated optee
    argument at program entry, aligning with standard Cargo custom subcommand
    conventions.

  • Introduce resolve_project_path to properly resolve and normalize the
    project path into an absolute path.

  • Unify directory switching logic in both ca_builder and ta_builder
    by using a shared ChangeDirectoryGuard from the common module.

  • Centralize target configuration handling in the common.rs crate and replace
    ad-hoc logic with a strongly-typed enum-based design. A unified
    get_target_and_cross_compile function is introduced for target resolution.

  • Add a shared get_target_directory_from_metadata helper in the common
    module to consistently derive the Cargo target directory.

  • Introduce join_and_check and join_format_and_check helpers to safely
    join and validate filesystem paths.

  • Provide a unified get_package_name helper in the common module for
    consistent package name resolution across modules.

  • Replace boolean flags with a strongly-typed ComponentType enum to improve
    clarity and extensibility.

  • Reorganize functions across modules following a top-down (call-order)
    layout to improve code readability.

  • Fix CI failures in std mode by adjusting the Cargo.toml configuration of
    std-only examples.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 33 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +313 to +317
.find(|pkg| {
pkg.manifest_path
.to_string()
.contains(&*cargo_toml_path_str)
})
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package lookup logic for workspace projects uses a substring match (contains) which could incorrectly match packages with similar path names. For example, if you have paths like "/project/my_ta/Cargo.toml" and "/project/my_ta_test/Cargo.toml", searching for "my_ta/Cargo.toml" could incorrectly match "my_ta_test/Cargo.toml". Consider using exact path comparison or comparing canonicalized paths instead.

Suggested change
.find(|pkg| {
pkg.manifest_path
.to_string()
.contains(&*cargo_toml_path_str)
})
.find(|pkg| pkg.manifest_path.to_string() == cargo_toml_path_str)

Copilot uses AI. Check for mistakes.
@apache apache deleted a comment from Copilot AI Jan 14, 2026
Copy link
Contributor

@ivila ivila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with 2 minor changes needed.

I think we can merge it after the changes are made. This PR has been open for a long time, so we can ship the tool first, let developers try it, get feedback, and gradually improve it. What's your opinion? @DemesneGH

@DemesneGH
Copy link
Contributor

DemesneGH commented Jan 14, 2026

I think we can merge it after the changes are made. This PR has been open for a long time, so we can ship the tool first, let developers try it, get feedback, and gradually improve it.

@ivila Sure, I think it's good to merge it before the next release (around Jan 30). I will quickly review it these days.

@DemesneGH
Copy link
Contributor

Once the remaining comments resolved, I believe this PR is ready to merge. The tool is now feature-complete for building both TA and CA, the CI passes, and the documentation is in place. I think the current version is sufficient for the first publish.

Regarding the goal of user interface design "the cargo-optee build command should provide a seamless interface aligned with the existing cargo build workflow" mentioned by @m4sterchain , I suggest to track that via a new Issue where we can perform the comparison with cargo and cargo-sgx, discuss and plan the next steps without delaying this initial merge.

@asir66
Copy link
Contributor Author

asir66 commented Jan 15, 2026

Summary of Changes

This refactors the configuration resolution system in cargo-optee and improves documentation.

Code Refactoring

  1. Decoupled configuration resolution logic

    • Moved TaBuildConfig and CaBuildConfig resolution logic to config.rs module
    • Separated metadata parsing from priority resolution (CLI > metadata > default)
    • Created MetadataConfig struct with resolve() method to handle metadata-only parsing
    • Removed redundant extract_build_config_from_metadata function
    • MetadataConfig::resolve() only parses metadata without handling priorities
  2. Improved code organization

    • Consolidated build configuration resolution in a single module
    • Better separation of concerns between metadata parsing and configuration resolution

Documentation Improvements

  1. Formatted documentation files

    • Formatted emulate-and-dev-in-docker-std.md and emulate-and-dev-in-docker.md
  2. Refactored README.md of cargo-optee

    • Improved Quick Start section for better clarity
    • Enhanced Configuration System section with clearer examples
  3. Removed hardcoded configurations

    • Removed hardcoded values from hello_world-rs example's Cargo.toml

@DemesneGH
Copy link
Contributor

Thanks for the follow-up commits!
The refactoring significantly improves the modularity: CLI params => TaBuildConfig::resolve() => TaBuilder::build(config). This separation of logic makes it much easier to maintain.
LGTM and will merge it after CI passes, thanks!

@asir66
Copy link
Contributor Author

asir66 commented Jan 16, 2026

Thank you for your review @DemesneGH @ivila

@DemesneGH DemesneGH merged commit 4a4322b into apache:main Jan 16, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants